Fix logout race: clear partitioned cookies, recheck session on 401 - #4701
Conversation
…cated Logout deleted the framework session cookie without the Partitioned attribute it was set with. Under CHIPS that empties the unpartitioned jar, so the cookie survived logout and any instance whose session-email cache still held the revoked token answered as the previous account. Cookie clearing now mirrors crossSiteCookieAttrs, and the decision lives in deleteCookieFromEveryScope so no caller can under-specify it. A 401 from an action also re-resolves the session, so a torn-down session redirects to sign-in instead of painting a generic load error.
|
@builderio-bot look at the latest PR feedback and fix anything you agree with. Be skeptical. Reply on each comment thread whether you fixed it and why. Get CI green and keep the branch mergeable. |
…cf4fb118dee941018a53
Mirroring crossSiteCookieAttrs closed the partitioned jar but opened the other direction: a cookie stored before CHIPS, or over plain HTTP on a host later served over HTTPS, survives a Partitioned-only delete. h3 dedupes set-cookie on name/domain/path and ignores Partitioned, but only evicts consistently for host-only cookies - with a Domain present the scan side fails to match at all. Emit both variants and put the unpartitioned one back only when h3 actually dropped it.
|
There was a problem with your request, please try again later. Error id: |
|
dispatch preview: Open preview Built by GitHub Actions from |
|
design preview: Open preview Built by GitHub Actions from |
|
analytics preview: Open preview Built by GitHub Actions from |
|
assets preview: Open preview Built by GitHub Actions from |
|
content preview: Open preview Built by GitHub Actions from |
|
slides preview: Open preview Built by GitHub Actions from |
|
forms preview: Open preview Built by GitHub Actions from |
|
mail preview: Open preview Built by GitHub Actions from |
|
calendar preview: Open preview Built by GitHub Actions from |
|
fw preview: Open preview Built by GitHub Actions from |
CI status on
|
|
@builderio-bot Get CI green and keep the branch mergeable. |
…cf4fb118dee941018a53
There was a problem hiding this comment.
Builder reviewed your changes — looks good ✅
Review Details
Incremental Code Review Summary
The latest PR head contains no new code changes that introduce a confirmed defect. The partition cleanup workaround remains sound: it emits both partitioned and unpartitioned deletion variants, handles host-only and configured-domain scopes, and preserves Set-Cookie ordering so a fresh session token is not removed by a trailing delete. The client-side 401 revalidation remains throttled, avoids 403-induced sign-outs, and is suppressed after sign-out begins.
Two independent review agents found no new actionable issues. The previously reported unpartitioned-cookie issue was already fixed and resolved in the prior review. Focused regression tests were reported passing by the agents; the broader auth suite remains affected by an existing incomplete database mock/environment warnings.
Risk level remains high because this PR changes authentication and session invalidation behavior. Browser verification was attempted, but all executors lacked Chrome automation tools. The dev server/application endpoints were healthy, but no UI evidence could be captured.
🧪 Browser testing: Attempted after this review — dev server healthy, but Chrome automation tools were unavailable in the executor environment.
|
There was a problem with your request, please try again later. Error id: |
Summary
Fixes the intermittent logout race where a user is briefly bounced to sign-in, then reappears signed in as the previous account with a "Something went wrong" error on the listing page.
Problem
After logout, the framework session cookie was deleted without matching the
Partitioned/SameSite=Noneattributes it was originally set with under CHIPS. A partition-blind delete removes the wrong jar, so the browser keeps sending the old session cookie. If an instance's cached session state still resolves that stale token, the app can briefly re-authenticate as the previous account. Separately, when a request came back401(server no longer recognizes the browser), nothing told the client session gate, so the app shell stayed mounted on a stale "authenticated" answer and data queries surfaced a bare "Something went wrong" instead of redirecting to sign-in.Solution
Partitioned) used when the cookie was set, so both the partitioned and unpartitioned jars are actually cleared.401, letting the client re-resolve the true session state and redirect to sign-in instead of showing a generic error.Key Changes
deleteCookieFromEveryScopenow appliescrossSiteCookieAttrs(event)so cookie clears match the scope/partition they were set in;clearFrameworkSessionHintCookiesupdated accordingly.recheckSessionAfterUnauthorized()inuse-session.ts: throttled (5s min interval) re-fetch of session state, skipped oncesigningOuthas begun to avoid resurrecting a session mid-logout.use-action.tscallsrecheckSessionAfterUnauthorized()on401responses (but not403, which represents an authenticated caller being refused one specific action).401vs. no-op on403, throttling of repeated re-checks, and re-check being a no-op once sign-out has started.@agent-native/coredescribing the cookie-partition fix and 401 re-resolution behavior.Review follow-up (
f6a26f72b)Reviewer flagged that mirroring
crossSiteCookieAttrsclosed the partitioned jar but opened the mirror-image gap: aPartitioned-only delete cannot remove a cookie stored before CHIPS, or over plain HTTP on a host later served over HTTPS. Agreed and fixed — both variants are now emitted.That needed a workaround. h3 dedupes
set-cookieonname;domain;pathand ignoresPartitioned, but only sometimes: it computes the key fromoptionswhen writing and re-parses the header when scanning, and the scan side never recoversdomain. Probed directly:Domain, differing partitionDomaindeleteCookieFromBothPartitionsre-appends the unpartitioned delete only when h3 actually evicted it, so it is correct under either behaviour.The middle row is pre-existing —
0dc7290a3emits the same shape — and harmless, because a browser appliesSet-Cookiein order and the set is last. Deliberately not fixed here; asserted instead, so no future change can leave a delete after the set. That assertion caught a duplicate domain-scoped delete in my first attempt at this follow-up.To clone this PR locally use the Github CLI with command
gh pr checkout 4701You can tag me at @BuilderIO for anything you want me to fix or change